Port #[custom_mir(..)] to the new attribute system
#145206
Conversation
|
|
rustbot
added
A-attributes
|
Some changes occurred in compiler/rustc_attr_parsing Some changes occurred in compiler/rustc_hir/src/attrs Some changes occurred in compiler/rustc_passes/src/check_attr.rs |
This comment has been minimized.
This comment has been minimized.
| let (dialect, span) = dialect?; | ||
|
|
||
| let dialect = match dialect.as_str() { | ||
| "analysis" => MirDialect::Analysis, |
There was a problem hiding this comment.
I think you can match directly on symbols here, so sym::analysis
|
|
||
| let phase = match phase.as_str().to_ascii_lowercase().as_str() { | ||
| "initial" => MirPhase::Initial, | ||
| "post_cleanup" | "post-cleanup" | "postcleanup" => MirPhase::PostCleanup, |
There was a problem hiding this comment.
Did the original impl have 3 variants? This feels like something that should be specified and then changed to one of these 3
|
|
||
| #[derive(Clone, Copy, Decodable, Debug, Encodable, PartialEq)] | ||
| #[derive(HashStable_Generic, PrintAttribute)] | ||
|
|
There was a problem hiding this comment.
How did i end up inserting a newline here?!
| return; | ||
| }; | ||
|
|
||
| let string_of_dialect = || match dialect { |
There was a problem hiding this comment.
you could implement a trait that allows these to be used in diagnostics directly. I forgot which, maybe IntoDiagnosticsValue? Something like that. It's from rustc_errors
There was a problem hiding this comment.
Inherent method name is usually a simple enough way to do that.
compiler/rustc_middle/src/mir/mod.rs
Outdated
| } | ||
| } | ||
| } | ||
|
|
||
| impl RuntimePhase { | ||
| pub fn parse(phase: Option<String>) -> Self { | ||
| pub fn parse(phase: Option<attrs::MirPhase>) -> Self { |
There was a problem hiding this comment.
Should this method be inlined into its use sites now that it's so simple? Like the other *Phase::parse?
| return; | ||
| }; | ||
|
|
||
| let string_of_dialect = || match dialect { |
There was a problem hiding this comment.
Inherent method name is usually a simple enough way to do that.
|
@cjgillot yea I told sasha off-github the same yesterday. Let's just collapse all those alternate syntaxes into one and make them symbols. :) |
|
(i'm kinda busy trying to unbreak the tests, i'll go through all these discussions once i have something that works (i was convinced i ran the tests before opening the PR, but apparently no --')) |
scrabsha
force-pushed
the
push-uxovoqzrxnlx
branch
3 times, most recently
from
22677db to
4acaec2
Compare
|
@cjgillot @jdonszelmann I think I addressed everything you raised in this PR :) |
This comment has been minimized.
This comment has been minimized.
scrabsha
force-pushed
the
push-uxovoqzrxnlx
branch
from
4acaec2 to
e854013
Compare
|
awesome, looks good! @bors r+ rollup |
bors
added
S-waiting-on-bors
…szelmann Port `#[custom_mir(..)]` to the new attribute system r? `@jdonszelmann`
…szelmann Port `#[custom_mir(..)]` to the new attribute system r? ``@jdonszelmann``
…szelmann Port `#[custom_mir(..)]` to the new attribute system r? ```@jdonszelmann```
bors
added
S-waiting-on-author
scrabsha
force-pushed
the
push-uxovoqzrxnlx
branch
from
e854013 to
17d11f6
Compare
|
@rustbot ready |
rustbot
added
S-waiting-on-review
|
@bors r+ |
bors
added
S-waiting-on-bors
…szelmann Port `#[custom_mir(..)]` to the new attribute system r? `@jdonszelmann`
bors
added
S-waiting-on-author
jdonszelmann
added
S-waiting-on-review
scrabsha
force-pushed
the
push-uxovoqzrxnlx
branch
from
17d11f6 to
a42cc37
Compare
|
@bors r+ (the other one has had some delays) |
bors
added
S-waiting-on-bors
|
@bors r- |
bors
added
S-waiting-on-author
|
Apparently #145085 is ready - I'll rebase this PR once it is merged :D |
r? @jdonszelmann